Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix #1055 Unexpected ROUNDDOWN function Result #1110

Merged
merged 5 commits into from
Jul 9, 2023
Merged

fix #1055 Unexpected ROUNDDOWN function Result #1110

merged 5 commits into from
Jul 9, 2023

Conversation

deleteJ
Copy link
Contributor

@deleteJ deleteJ commented Jun 29, 2023

this closes #1055

  1. use decimal to replace double on precision issue
  2. add a unit test file to track the issue
    image

@tonyqus tonyqus changed the title fix issue #1055 unexpected ROUNDDOWN function Result, use Decimal to fix fix #1055 Unexpected ROUNDDOWN function Result Jun 29, 2023
@tonyqus tonyqus added this to the NPOI 2.7.0 milestone Jun 29, 2023
@deleteJ
Copy link
Contributor Author

deleteJ commented Jun 30, 2023

  1. seems like it breaks 1 test cases, change left side to decimal or remove this check?

image

left side is double precision, while right side is formula decimal precision
Assert.AreEqual(NEW_PART_COST * NEW_QUANT, ccell.NumericCellValue);

image

  1. other build fail no clue yet

@deleteJ deleteJ closed this Jun 30, 2023
@deleteJ deleteJ reopened this Jun 30, 2023
@lahma
Copy link
Collaborator

lahma commented Jun 30, 2023

This might just be a case of Linux font creating a different width. It's terrible, I know. You can check for platform in test case and adjust when needed. Of course it's a bit worrying that different platforms will create different representation - if they do.

npoi/testcases/main/HSSF/UserModel/TestHSSFSheet.cs

Lines 858 to 860 in 32435d9

// there's slight difference due to fonts
var expectedRowHeight = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 506 : 528;
Assert.AreEqual(expectedRowHeight, row.Height);

@deleteJ
Copy link
Contributor Author

deleteJ commented Jun 30, 2023

thanks Lahma to confirm, I just noticed this ubuntu column check fail not introduced by me...

good ending, I can go sleep now

@lahma
Copy link
Collaborator

lahma commented Jun 30, 2023

Please have a good night's sleep🙂 It's still problematic to merge though, as it would break master build.

@tonyqus
Copy link
Member

tonyqus commented Jul 9, 2023

Since Windows test running is pass, I'd like to merge it first.

@tonyqus
Copy link
Member

tonyqus commented Jul 9, 2023

LGTM

@tonyqus tonyqus merged commit e8ec7a3 into nissl-lab:master Jul 9, 2023
1 of 2 checks passed
@tonyqus tonyqus modified the milestones: NPOI 2.7.0, NPOI 2.6.2 Aug 23, 2023
@Bykiev Bykiev mentioned this pull request Sep 19, 2023
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug formula
Projects
None yet
Milestone
NPOI 2.6.2
Development

Successfully merging this pull request may close these issues.

Unexpected ROUNDDOWN function Result
3 participants
@deleteJ @lahma @tonyqus